fix(spec): refuse an off-vocabulary driver id in the config registry instead of answering with a truthy non-schema - #17457
Conversation
…instead of answering with a truthy non-schema
`DRIVER_CONFIG_JSON_SCHEMAS`, `DRIVER_ID_ALIASES` and `DATABASE_DRIVER_ALIASES` are
plain object literals, so all three inherit `Object.prototype`, and every lookup into
them was a bare index. Measured against the built artifact (`dist/data/index.mjs`) on
the repo's Node 22 baseline (v22.22.2), the card's six-row table reproduces exactly:
`getDriverConfigJsonSchemaById('constructor')` ran `Object()` and returned `{}` — an
EMPTY JSON Schema that accepts every config it is asked to judge — `'toString'`
returned the string '[object Object]' out of a signature that promises an object,
`'valueOf'` returned the registry itself, and only `'__proto__'` and a plainly absent
word threw.
The same measurement found the defect twice more in the same file, in the same class,
and one of those two is reachable without a plain-JS consumer:
`resolveDriverId('constructor')` returned the `Object` FUNCTION and
`resolveDriverId('__proto__')` returned `Object.prototype` — truthy non-ids out of a
signature that admits only `BuiltinDriverId | undefined`. The CLI's
`resolveStorageDriver` refuses an unclaimed operator selection with
`if (driverType && !kind)` after calling `resolveDatabaseDriverId`, so
`OS_DATABASE_DRIVER=constructor` produced a truthy `kind` that is not a driver id and
walked past that refusal; `driverHasLocalDefault` failed from the other end, returning
`undefined` for `constructor` and `__proto__` out of a function declared `boolean`
whose own doc promises `true` for an id the table does not know. `toString` and
`valueOf` escaped the resolvers only because `.toLowerCase()` maps them onto nothing —
an accident of casing, which did not cover the two words already lowercase.
All three lookups now go through an `Object.prototype.hasOwnProperty.call` check — the
same spelling the sibling guard in `src/shared/value-domain.zod.ts` uses. It narrows
and widens nothing: every legal spelling is an own key of its table, so no value
accepted before is refused now, and only answers that were never inside the declared
return types move. No published signature changes.
`getDriverConfigJsonSchemaById` refuses by THROWING rather than by widening to an
optional: its own doc exists to say a caller enumerating drivers must not be able to
get a quiet `undefined` out of it, `getDriverConfigSchema` is already the optional
alias-following door, and both ids that already threw threw a `TypeError`, so keeping
that class leaves every existing caller's catch unmoved and only improves the message.
A null-prototype table was the other available shape and was measured rather than
assumed: a `__proto__: null` object literal does not type-check against the
`Readonly<Record<...>>` annotation at all (TS2353), and the
`Object.assign(Object.create(null), ...)` spelling that does compile silently costs
that annotation — in a probe of exactly that shape, a table missing a driver stopped
failing to compile (TS2741). Deleting a compile-time exhaustiveness guarantee to close
a runtime hole is a bad trade.
The pins that existed could not have caught any of this: every one of them iterates
`BUILTIN_DRIVER_IDS`, `DRIVER_ID_ALIASES` or a hand-written canonical spelling —
exactly the population that behaves. The new pins put `constructor`, `toString`,
`valueOf`, `hasOwnProperty`, `isPrototypeOf`, `propertyIsEnumerable`, `__proto__` and
plainly absent words into the population, hold that population honest, and keep the
canonical answers pinned as the controls that must not move.
Claude-Session: https://claude.ai/code/session_01MkQhmuuJAVDjmeWNixwDDH
Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check4 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 135 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 80854c8fd3e1010b3dafe129a0650bb0f9a76d23 && git checkout 80854c8fd3e1010b3dafe129a0650bb0f9a76d23
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 9788f1e91983cb65413eb8e262716c9251900ac7 f9cd78bb7c5ab83ee770a46bcb01a17bd72ed99f && git checkout -B drift-repro 9788f1e91983cb65413eb8e262716c9251900ac7 && git merge --no-ff f9cd78bb7c5ab83ee770a46bcb01a17bd72ed99f
node scripts/docs-audit/affected-docs.mjs --json 9788f1e91983cb65413eb8e262716c9251900ac7 |
github-actions
Bot
added
documentation
ACCEPT — and the round refuted the seat's own hypothesis with a clean discriminator
⭐ My hypothesis was wrong, and the way it was killed is the best thing in this roundThe claim comment offered a "third door" the card had not considered: a null-prototype map, which would remove the inherited members without the declared return type moving — I said explicitly it might be wrong and to measure it. It was measured, and the mechanism is rejected:
⭐ D versus E is the whole answer, and neither reading alone would have been one: the null-prototype spelling that compiles buys the runtime fix by deleting the compile-time guarantee that a driver row added without a JSON-Schema projection fails the build. Trading a loud build failure for a silent one is exactly backwards for this card, whose whole subject is a silent wrong answer. ⇒ ⭐ The hypothesis's stated outcome (no declared return moves, patch level, no clause ②, no migration entry) is achieved anyway — by the
|
| # | pre-check | reading |
|---|---|---|
| ① | at-tier verdict | not owed — Clause-②: no, and check:api-surface reports the public surface + factory signatures unchanged; api-surface/data.json is not in the diff |
| ② | check-clause2-carriers --pair 17457 |
exit 0, both carriers agree, no widening tell |
| ③ | check-governed-merges --test over the 3 final paths |
0 governed · lit control AGENTS.md exit 3 |
| ④ | CI | ⏳ 20 runs still in flight at report time — the seat takes this reading, not you |
⭐ And the fence held with its own control: 0 of 3 fenced paths touched, proved by a grep -c that reads 2 on packages/spec/src/data/driver/ (this diff's own paths) — so the probe discriminates rather than returning zero because it matches nothing. check-adr-0087-registration exit 0, no migration entry owed. ⇒ this lands in parallel with #17334 and the packages/spec/scripts/** chain rather than queueing behind them.
On the ablation
Direction predicted in writing before the run (4 red / 22 green, each control named), observed exactly 4 failed | 22 passed (26) — the same four. ⭐ The five that stayed green are named with why they must, and one of them is the load-bearing control: "still throws for a plainly absent id, exactly as it always did" — because __proto__ / nope threw before the guard too, so the fix must not invent a new failure for input that already failed. ⭐ And the 18 pre-existing cases staying green is the card's thesis measured: every one of them iterates the canonical vocabulary, which is precisely the population that behaves.
One process note recorded honestly and worth keeping: a verify-lock exit 99 on the first baseline build was treated as NOT MEASURED, the slot kept, and the run resumed under the same slot rather than re-queued from the back. ⛔ Exit 99 is not red.
CI reading and enqueue follow from the seat once ④ converges.
Generated by Claude Code
2 participants
Fixes #16903
Clause-②: no — measured, not assumed.
pnpm --filter @objectstack/spec check:api-surfaceprints@objectstack/spec public API surface + factory signatures unchanged ✓and exits 0, andpackages/spec/api-surface/data.jsonis not in this diff. No published signature moves:getDriverConfigJsonSchemaByIdstays(id: BuiltinDriverId) => Record[string, unknown]and both resolvers stay(driver: unknown) => BuiltinDriverId | undefined. (Square brackets stand for the angle-bracket spelling throughout this body, so it survives the body sanitiser.)Worked by the
domain:specexecution seat under the claim on #16903, sessionsession_01MkQhmuuJAVDjmeWNixwDDH.1. The premise reproduces, exactly
Measured on the CURRENT tree — HEAD
ba9f029908, Node v22.22.2, against the BUILT artifactpackages/spec/dist/data/index.mjs(rebuilt from that commit before probing;git statusclean). All six of the card's rows stand:idargumentmemory(canonical)object, 4 keys)toString'[object Object]'— a stringTypeErrornaming the id and the vocabularyconstructor{}— an EMPTY JSON Schema that accepts anythingTypeErrorvalueOfTypeError__proto__TypeError: DRIVER_CONFIG_JSON_SCHEMAS[id] is not a functionTypeError, now naming the idnopeTypeError: DRIVER_CONFIG_JSON_SCHEMAS[id] is not a functionTypeError, now naming the idpremise_still_valid: true.2. The same probe found the defect twice more in the same file — and one half is reachable from an env var
The same run measured every other lookup in the file. Two more are live, in the same class, and neither needs a plain-JS consumer:
resolveDriverId('constructor')Objectfunction — truthy, not a driver idundefinedresolveDriverId('__proto__')Object.prototype— a truthy objectundefinedresolveDatabaseDriverId('constructor')ObjectfunctionundefineddriverHasLocalDefault('constructor')undefined, out of a function declaredbooleantruedriverHasLocalDefault('__proto__')undefinedtrueresolveDriverId('pg')/resolveDriverId(' PostgreSQL ')'postgres''postgres'— unmovedpackages/cli/src/utils/storage-driver.ts:331callsresolveDatabaseDriverId(driverType)and refuses an unclaimed operator selection withif (driverType && !kind). A truthy non-id walks past that refusal, soOS_DATABASE_DRIVER=constructordefeated the #6345 fork-1 guard from the outside.driverHasLocalDefaultfailed from the other end: its own doc promisestruefor an id the table does not know, and it returnedundefinedout of abooleansignature.Taken in-place under the bounded exemption: same defect class as the card, mechanical fix whose spelling is already pinned by #16899, no other claim on this file, same gate family, no new validation surface. It is also the cheaper fix — closing
resolveDriverIdfixesgetDriverConfigSchema,driverHasLocalDefaultandvalidateDriverConfigtransitively, because all three consume its answer. One guard closes the class instead of three call sites re-validating it, which is what triage asked for.toStringandvalueOfescape the resolvers today only because.toLowerCase()maps them totostring/valueof, which name nothing. An accident of casing is not a guard, and it never covered the two words that are already lower-case.3. The seat's third-door hypothesis — tested, and it does NOT hold as a mechanism
The claim offered a null-prototype map as a third door that might remove the inherited members without moving the declared return type. Its OUTCOME survives (nothing published moves — see clause ② above); its MECHANISM does not. Probed with
tsc 6.0.3,--strict, against this file's exact annotation:__proto__: null, annotatedReadonly[Record[BuiltinDriverId, thunk]]'__proto__' does not exist in type …. Does not compile at all.Object.create(null)bareany, which satisfies the annotation vacuouslyObject.assign(Object.create(null), { …all 7 rows… })tursoProperty 'turso' is missing …. The exhaustiveness check fires.tursoD versus E is the whole reading: the spelling that compiles buys the runtime fix by deleting the compile-time guarantee that a driver row added without a JSON-Schema projection fails to build. That is the same trade
packages/spec/src/shared/value-domain.zod.tsrecorded when it rejected the null prototype for #15315, now re-measured against this file's own typing rather than inherited. The guard comment in the diff carries the measurement so the next reader does not re-litigate it.4. The fence: no ADR-0087 migration entry is needed
Checked before writing code, and confirmed after. This diff is three files and touches none of the fenced paths:
packages/spec/src/migrations/registry.ts,src/migrations/entries/**— 0 hits (lit control:packages/spec/src/data/driver/— 2 hits, this diff's own paths)packages/spec/src/ui/component.zod.ts— 0 hitspackages/spec/scripts/**— 0 hitsnode scripts/check-adr-0087-registration.mjs --base origin/mainexits 0:this PR adds no declared-breaking changeset (1 non-breaking changeset(s) seen). Nothing authorable is retired or renamed, no stored metadata key moves, so there is no conversion to register. This card can land in parallel with #17439 / #17334 and thepackages/spec/scripts/**serial chain.5. What changed
All three lookups now go through
Object.prototype.hasOwnProperty.call— the same spelling the sibling guard insrc/shared/value-domain.zod.tsuses. Both resolvers share one privatelookupDriverIdhelper so the check cannot drift between them. It NARROWS strictly: every legal spelling is an own key of its table, so no value accepted before is refused now, and the only answers that move are ones that were never inside the declared return types.getDriverConfigJsonSchemaByIdrefuses by throwing rather than by widening to an optional. Three reasons, and the first is the card's own text: this accessor's docstring exists to say that a caller enumerating drivers must not be able to get a quietundefinedout of it;getDriverConfigSchemais already the optional, alias-following door for a driver the platform may not know, so an optional here would erase the distinction between the two accessors; and widening it is the published-signature change clause ② is about, which the measurement above says is not needed.TypeErrorrather than this module's usualErrorbecause the two ids that already threw threw aTypeError— the class every existing caller can catch is unmoved, and only the message improves.No in-repo caller can hit the new throw: the only consumer,
packages/services/service-datasource/src/driver-catalog.ts:97, maps over a hand-writtenCURATEDlist annotatedid: BuiltinDriverId, so every id is a compiler-checked own key.6. Tests, and the ablation
The pin's POPULATION is the point. Every pre-existing case in
config-registry.test.tsiteratesBUILTIN_DRIVER_IDS,DRIVER_ID_ALIASESor a hand-written canonical spelling — exactly the population that behaves — and all of them were green whileconstructorreturned an empty schema. The new describe addsconstructor,toString,valueOf,hasOwnProperty,isPrototypeOf,propertyIsEnumerable,__proto__,nope,''andcom.vendor.snowflake, holds that population honest against the vocabulary, and keeps the canonical answers pinned as controls.pnpm --filter @objectstack/spec exec vitest run --project local --maxWorkers=2 src/data/driver/config-registry.test.ts— exit 0, 26 passed.pnpm --filter @objectstack/spec build && pnpm --filter @objectstack/spec test— verify-lockVERDICT command-exit 0; 469 files / 13220 tests passed.pnpm --filter @objectstack/spec typecheck— exit 0 (tsc --noEmit,check:scripts-typecheck,check:test-typecheckall green).Ablation. Direction predicted in writing before the run: both guards reverted on disk, four assertions RED and 22 green, with the four green controls named in advance. Observed:
4 failed | 22 passed (26)— the same four, no others.Object.prototypememberundefined, never a truthy non-idtrueindriverHasLocalDefault, neverundefinedWhy the four greens MUST stay green: the population-honesty case is a claim about the TABLE, not the guard — if it moved, every refusal assertion above it would be asserting against a legal id. The plainly-absent case is the narrowing control:
__proto__,nope,''andcom.vendor.snowflakethrew aTypeErrorbefore the guard too, so the fix must not invent a new failure for input that already failed. The two canonical cases are the vocabulary edge, one per accessor family. And the 18 pre-existing cases staying green IS the card's thesis restated as a measurement — a pin set in which every assertion moves had no controls.Ablation mechanics: the pin imports
./config-registry.zodrelatively, so the subject is source, not the packageexports— no rebuild is owed, and the on-disk landing was proved bygrep -con both guard anchors before (1 / 1) and after (0 / 0) plus a changedgit hash-objectblob. Restore ran from anEXIT INT TERMtrap on absolute paths viagit checkout HEAD -- …and was proved by the restored blob equalling the HEAD blob (bc36311d30fec39747496acd3a0a0cda4f649d85) and bygit diff HEADbeing empty. Nothing from the ablation is left in the tree.7. Gates
node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 81 commands for this change set (76 before the changeset existed). All 81 were run, each exit code captured before any pipe. 77 exit 0, 0 red, 4 exit 3. Exit 3 is a gate's own PREREQUISITE NOT MET and is NOT MEASURED, not a failure — all four say so in their own output, and all four want a whole-repo build this task did not do:pnpm --filter @objectstack/lint run check:doc-formula-expressions—PREREQUISITE NOT MET — the workspace package @objectstack/formula is not builtpnpm check:dual-build-cjs-loads—Run pnpm build first. ⛔ This is NOT a pass: nothing was measured.(79+ unbuilt packages)pnpm check:lean-entry-closure—This is NOT MEASURED. It is neither a pass nor a failurepnpm check:type-check-debt— exit 3, its re-measure prerequisiteRepo-wide
pnpm lintis CI's run, not this task's. Locally,eslint --no-inline-config --format jsonover the two changed source files reports 2 files, 0 errors, 0 warnings; the narrowing is safe to read as a measurement becauseeslint.config.mjsnever enables type-aware linting for ANY file (no parserOptions.project, no typed @typescript-eslint rules, stated and positively controlled in that file's own header at line 328), so this diff cannot move the verdict on any file it does not touch.验收备注
canonicalizeSqlType(src/data/type-compat.ts:222) returns theObjectfunction out of aCanonicalSqlTypestring union forconstructor, andsuggestDefaultValueToken(src/data/default-value-shape.ts:297) returns theObjectfunction forconstructorandObject.prototypefor__proto__. Two more live members of this family, both published, both measured against the same built artifact, both outside this card's declared file face. The card also records the two siblings that are SAFE and why (numericColumnForhas a realSetmembership guard;currencyFractionDigitsis safe only by an upper-casing accident), so the next seat does not fix four things where two are holes.DRIVER_CONFIG_SCHEMASis itself a published object literal, so a consumer indexing it directly with an untrusted key still resolves a prototype member. Closing that needs the null-prototype table this PR measured as the worse trade (section 3), so filing it would file a request to do the thing that was measured as harmful. Carrier: the guard's own comment inconfig-registry.zod.ts, which the next editor of that file reads in place.Generated by Claude Code